fix: try to correct mtime on upsyncs

Files with a modification time of less than 0 do usually not make sense
(and afaik the server doesn't accept them either).
--> attempt to update the modification time to _Time.now_ while
propagating

side note: I ran into this because KArchive/Ark(?) didn't consider the
extra time attributes on entries for a certain zip file, so it instead
used the standard time value of each zip entry which was set to <= 1980
for files and < 1970 for directories...

Signed-off-by: Jyrki Gadinger <nilsding@nilsding.org>

diff --git a/src/libsync/bulkpropagatorjob.cpp b/src/libsync/bulkpropagatorjob.cpp
index 637e46c9a6181b8a340669670b26b0e41d3788ee..942605f18f45a09724685250e4c3198ce4f7fd38 100644 (file)
--- a/src/libsync/bulkpropagatorjob.cpp
+++ b/src/libsync/bulkpropagatorjob.cpp
@@ -171,10 +171,17 @@ void BulkPropagatorJob::doStartUpload(SyncFileItemPtr item,
 
         item->_modtime = FileSystem::getModTime(newFilePathAbsolute);
         if (item->_modtime <= 0) {
-            _pendingChecksumFiles.remove(item->_file);
-            slotOnErrorStartFolderUnlock(item, SyncFileItem::NormalError, tr("File %1 has invalid modified time. Do not upload to the server.").arg(QDir::toNativeSeparators(item->_file)), ErrorCategory::GenericError);
-            checkPropagationIsDone();
-            return;
+            const auto now = QDateTime::currentSecsSinceEpoch();
+            qCInfo(lcPropagateUpload) << "File" << item->_file << "has invalid modification time of" << item->_modtime << "-- trying to update it to" << now;
+            if (FileSystem::setModTime(newFilePathAbsolute, now)) {
+                item->_modtime = now;
+            } else {
+                qCWarning(lcPropagateUpload) << "Could not update modification time for" << item->_file;
+                _pendingChecksumFiles.remove(item->_file);
+                slotOnErrorStartFolderUnlock(item, SyncFileItem::NormalError, tr("File %1 has invalid modified time. Do not upload to the server.").arg(QDir::toNativeSeparators(item->_file)), ErrorCategory::GenericError);
+                checkPropagationIsDone();
+                return;
+            }
         }
     }
 
@@ -298,18 +305,26 @@ void BulkPropagatorJob::slotStartUpload(SyncFileItemPtr item,
         return;
     }
 
-    const auto prevModtime = item->_modtime; // the _item value was set in PropagateUploadFile::start()
+    const auto prevModtime = item->_modtime; // the item value was set in PropagateUploadFile::start()
     // but a potential checksum calculation could have taken some time during which the file could
     // have been changed again, so better check again here.
 
     item->_modtime = FileSystem::getModTime(originalFilePath);
+    qCDebug(lcPropagateUpload) << "fullFilePath" << fullFilePath << "originalFilePath" << originalFilePath << "prevModtime" << prevModtime << "item->_modtime" << item->_modtime;
     if (item->_modtime <= 0) {
-        _pendingChecksumFiles.remove(item->_file);
-        slotOnErrorStartFolderUnlock(item, SyncFileItem::NormalError, tr("File %1 has invalid modification time. Do not upload to the server.").arg(QDir::toNativeSeparators(item->_file)), ErrorCategory::GenericError);
-        checkPropagationIsDone();
-        return;
+        const auto now = QDateTime::currentSecsSinceEpoch();
+        qCInfo(lcPropagateUpload) << "File" << item->_file << "has invalid modification time of" << item->_modtime << "-- trying to update it to" << now;
+        if (FileSystem::setModTime(originalFilePath, now)) {
+            item->_modtime = now;
+        } else {
+            qCWarning(lcPropagateUpload) << "Could not update modification time for" << item->_file;
+            _pendingChecksumFiles.remove(item->_file);
+            slotOnErrorStartFolderUnlock(item, SyncFileItem::NormalError, tr("File %1 has invalid modification time. Do not upload to the server.").arg(QDir::toNativeSeparators(item->_file)), ErrorCategory::GenericError);
+            checkPropagationIsDone();
+            return;
+        }
     }
-    if (prevModtime != item->_modtime) {
+    if (prevModtime > 0 && prevModtime != item->_modtime) {
         propagator()->_anotherSyncNeeded = true;
         _pendingChecksumFiles.remove(item->_file);
 

diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp
index 9939d44739021de1c074426b0889c4fe76a26f7c..4c0dfa151f8d5f64a7b7041385aeebde164aeb5d 100644 (file)
--- a/src/libsync/discovery.cpp
+++ b/src/libsync/discovery.cpp
@@ -1121,6 +1121,7 @@ void ProcessDirectoryJob::processFileAnalyzeLocalInfo(
         }
 
         if ((item->_direction == SyncFileItem::Down || item->_instruction == CSYNC_INSTRUCTION_CONFLICT || item->_instruction == CSYNC_INSTRUCTION_NEW || item->_instruction == CSYNC_INSTRUCTION_SYNC) &&
+                item->_direction != SyncFileItem::Up &&
                 (item->_modtime <= 0 || item->_modtime >= 0xFFFFFFFF)) {
             item->_instruction = CSYNC_INSTRUCTION_ERROR;
             item->_errorString = tr("Cannot sync due to invalid modification time");

diff --git a/src/libsync/propagateupload.cpp b/src/libsync/propagateupload.cpp
index 4946f05e98d5065c2783f84a0f383e3bb95fe5bb..2bac80f172b893818fb56f269914f1217f9dd3e9 100644 (file)
--- a/src/libsync/propagateupload.cpp
+++ b/src/libsync/propagateupload.cpp
@@ -324,8 +324,15 @@ void PropagateUploadFileCommon::slotComputeContentChecksum()
     // probably temporary one.
     _item->_modtime = FileSystem::getModTime(filePath);
     if (_item->_modtime <= 0) {
-        slotOnErrorStartFolderUnlock(SyncFileItem::NormalError, tr("File %1 has invalid modification time. Do not upload to the server.").arg(QDir::toNativeSeparators(_item->_file)));
-        return;
+        const auto now = QDateTime::currentSecsSinceEpoch();
+        qCInfo(lcPropagateUpload) << "File" << _item->_file << "has invalid modification time of" << _item->_modtime << "-- trying to update it to" << now;
+        if (FileSystem::setModTime(filePath, now)) {
+            _item->_modtime = now;
+        } else {
+            qCWarning(lcPropagateUpload) << "Could not update modification time for" << _item->_file;
+            slotOnErrorStartFolderUnlock(SyncFileItem::NormalError, tr("File %1 has invalid modification time. Do not upload to the server.").arg(QDir::toNativeSeparators(_item->_file)));
+            return;
+        }
     }
 
     const QByteArray checksumType = propagator()->account()->capabilities().preferredUploadChecksumType();

diff --git a/test/syncenginetestutils.cpp b/test/syncenginetestutils.cpp
index 8eac8a06ecba1a68049e37758db106b2372c68fa..507fcd2eee0e54c3d4129d9f60f9050778ed7764 100644 (file)
--- a/test/syncenginetestutils.cpp
+++ b/test/syncenginetestutils.cpp
@@ -535,6 +535,7 @@ FakePutMultiFileReply::FakePutMultiFileReply(FileInfo &remoteRootFileInfo, QNetw
 
 QVector<FileInfo *> FakePutMultiFileReply::performMultiPart(FileInfo &remoteRootFileInfo, const QNetworkRequest &request, const QByteArray &putPayload, const QString &contentType)
 {
+    Q_UNUSED(request)
     QVector<FileInfo *> result;
 
     auto stringPutPayload = QString::fromUtf8(putPayload);
@@ -564,7 +565,7 @@ QVector<FileInfo *> FakePutMultiFileReply::performMultiPart(FileInfo &remoteRoot
             // Assume that the file is filled with the same character
             fileInfo = remoteRootFileInfo.create(fileName, onePartBody.size(), onePartBody.at(0).toLatin1());
         }
-        fileInfo->lastModified = OCC::Utility::qDateTimeFromTime_t(request.rawHeader("x-oc-mtime").toLongLong());
+        fileInfo->lastModified = OCC::Utility::qDateTimeFromTime_t(modtime);
         remoteRootFileInfo.find(fileName, /*invalidateEtags=*/true);
         result.push_back(fileInfo);
     }

diff --git a/test/testsyncengine.cpp b/test/testsyncengine.cpp
index b2d721aaaeccb76ce1292fd69e82db8ff7e04f33..d1d182c82b3f9150acaa7c39f1e54e4c9291faa1 100644 (file)
--- a/test/testsyncengine.cpp
+++ b/test/testsyncengine.cpp
@@ -1333,6 +1333,71 @@ private slots:
         QCOMPARE(fakeFolder.currentRemoteState(), expectedState);
     }
 
+    void testLocalInvalidMtimeCorrection()
+    {
+        const auto INVALID_MTIME = QDateTime::fromSecsSinceEpoch(0);
+        const auto RECENT_MTIME = QDateTime::fromSecsSinceEpoch(1743004783); // 2025-03-26T16:59:43+0100
+
+        FakeFolder fakeFolder{FileInfo{}};
+        QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
+
+        fakeFolder.localModifier().insert(QStringLiteral("invalid"));
+        fakeFolder.localModifier().setModTime("invalid", INVALID_MTIME);
+        fakeFolder.localModifier().insert(QStringLiteral("recent"));
+        fakeFolder.localModifier().setModTime("recent", RECENT_MTIME);
+
+        QVERIFY(fakeFolder.syncOnce());
+
+        // "invalid" file had a mtime of 0, so it's been updated to the current time during testing
+        const auto currentMtime = fakeFolder.currentLocalState().find("invalid")->lastModified;
+        QCOMPARE_GT(currentMtime, RECENT_MTIME);
+        QCOMPARE_GT(fakeFolder.currentRemoteState().find("invalid")->lastModified, RECENT_MTIME);
+
+        // "recent" file had a mtime of RECENT_MTIME, so it shouldn't have been changed
+        QCOMPARE(fakeFolder.currentLocalState().find("recent")->lastModified, RECENT_MTIME);
+        QCOMPARE(fakeFolder.currentRemoteState().find("recent")->lastModified, RECENT_MTIME);
+
+        QVERIFY(fakeFolder.syncOnce());
+
+        // verify that the mtime of "invalid" hasn't changed since the last sync that fixed it
+        QCOMPARE(fakeFolder.currentLocalState().find("invalid")->lastModified, currentMtime);
+
+        QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
+    }
+
+    void testLocalInvalidMtimeCorrectionBulkUpload()
+    {
+        const auto INVALID_MTIME = QDateTime::fromSecsSinceEpoch(0);
+        const auto RECENT_MTIME = QDateTime::fromSecsSinceEpoch(1743004783); // 2025-03-26T16:59:43+0100
+
+        FakeFolder fakeFolder{FileInfo{}};
+        QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
+        fakeFolder.syncEngine().account()->setCapabilities({ { "dav", QVariantMap{ {"bulkupload", "1.0"} } } });
+
+        fakeFolder.localModifier().insert(QStringLiteral("invalid"));
+        fakeFolder.localModifier().setModTime("invalid", INVALID_MTIME);
+        fakeFolder.localModifier().insert(QStringLiteral("recent"));
+        fakeFolder.localModifier().setModTime("recent", RECENT_MTIME);
+
+        QVERIFY(fakeFolder.syncOnce()); // this will use the BulkPropagatorJob
+
+        // "invalid" file had a mtime of 0, so it's been updated to the current time during testing
+        const auto currentMtime = fakeFolder.currentLocalState().find("invalid")->lastModified;
+        QCOMPARE_GT(currentMtime, RECENT_MTIME);
+        QCOMPARE_GT(fakeFolder.currentRemoteState().find("invalid")->lastModified, RECENT_MTIME);
+
+        // "recent" file had a mtime of RECENT_MTIME, so it shouldn't have been changed
+        QCOMPARE(fakeFolder.currentLocalState().find("recent")->lastModified, RECENT_MTIME);
+        QCOMPARE(fakeFolder.currentRemoteState().find("recent")->lastModified, RECENT_MTIME);
+
+        QVERIFY(fakeFolder.syncOnce()); // this will not propagate anything
+
+        // verify that the mtime of "invalid" hasn't changed since the last sync that fixed it
+        QCOMPARE(fakeFolder.currentLocalState().find("invalid")->lastModified, currentMtime);
+
+        QCOMPARE(fakeFolder.currentLocalState(), fakeFolder.currentRemoteState());
+    }
+
     void testServerUpdatingMTimeShouldNotCreateConflicts()
     {
         constexpr auto testFile = "test.txt";

diff --git a/test/testsyncvirtualfiles.cpp b/test/testsyncvirtualfiles.cpp
index f6281ab38df7143bf6d34f58abfd2e1f4b1d28a5..03e743ec31b86861c98386380b3666f6c1b53df6 100644 (file)
--- a/test/testsyncvirtualfiles.cpp
+++ b/test/testsyncvirtualfiles.cpp
@@ -1751,6 +1751,10 @@ private slots:
             return {};
         };
 
+        const auto lastModified = [&](const QString &path) -> qint64 {
+            return fakeFolder.currentLocalState().find(path)->lastModified.toSecsSinceEpoch();
+        };
+
         fakeFolder.localModifier().insert(fooFileRootFolder);
         fakeFolder.localModifier().insert(barFileRootFolder);
         fakeFolder.localModifier().mkdir(QStringLiteral("subfolder"));
@@ -1765,24 +1769,33 @@ private slots:
         fakeFolder.scheduleSync();
         fakeFolder.execUntilBeforePropagation();
 
-        QCOMPARE(checkStatus(), SyncFileStatus::StatusError);
+        QCOMPARE(checkStatus(), SyncFileStatus::StatusSync);
 
         fakeFolder.execUntilFinished();
 
+        // ensure mtime has changed after the sync
+        QCOMPARE_GT(lastModified(barFileAaaSubFolder), CURRENT_MTIME);
+
         fakeFolder.localModifier().setModTime(barFileAaaSubFolder, QDateTime::fromSecsSinceEpoch(CURRENT_MTIME));
 
         QVERIFY(fakeFolder.syncOnce());
 
+        // ensure mtime is now CURRENT_MTIME
+        QCOMPARE(lastModified(barFileAaaSubFolder), CURRENT_MTIME);
+
         fakeFolder.localModifier().appendByte(barFileAaaSubFolder);
         fakeFolder.localModifier().setModTime(barFileAaaSubFolder, QDateTime::fromSecsSinceEpoch(INVALID_MTIME1));
 
         fakeFolder.scheduleSync();
         fakeFolder.execUntilBeforePropagation();
 
-        QCOMPARE(checkStatus(), SyncFileStatus::StatusError);
+        QCOMPARE(checkStatus(), SyncFileStatus::StatusSync);
 
         fakeFolder.execUntilFinished();
 
+        // ensure mtime has changed after the sync
+        QCOMPARE_GT(lastModified(barFileAaaSubFolder), CURRENT_MTIME);
+
         fakeFolder.localModifier().setModTime(barFileAaaSubFolder, QDateTime::fromSecsSinceEpoch(CURRENT_MTIME));
 
         QVERIFY(fakeFolder.syncOnce());
@@ -1793,7 +1806,11 @@ private slots:
         fakeFolder.scheduleSync();
         fakeFolder.execUntilBeforePropagation();
 
-        QCOMPARE(checkStatus(), SyncFileStatus::StatusError);
+        QCOMPARE(checkStatus(), SyncFileStatus::StatusSync);
+
+        // the server only considers an mtime of 0-86400 (1d) as invalid, so this is fine
+        // see also: apps/dav/lib/Connector/Sabre/MtimeSanitizer.php
+        QCOMPARE(lastModified(barFileAaaSubFolder), INVALID_MTIME2);
 
         fakeFolder.execUntilFinished();
     }